Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add page on source code verification #6575

Merged
merged 13 commits into from
Jul 13, 2022
Merged

Conversation

emmanuel-awosika
Copy link
Contributor

Describes the process (and importance) of verifying source code for Ethereum smart contracts.

Description

The page explains what source code verification means, how it's done, why it matters for users and developers. It also has a section describing the tools for verifying contract code for Ethereum smart contracts.

Related Issue

#6308

Describes the process (and importance) of verifying source code for Ethereum smart contracts.
@gatsby-cloud
Copy link

gatsby-cloud bot commented Jun 6, 2022

Gatsby Cloud Build Report

ethereum-org-website-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 34m

Performance

Lighthouse report

Metric Score
Performance 🔶 19
Accessibility 💚 91
Best Practices 💚 100
SEO 💚 92

🔗 View full report

Copy link
Member

@kuzdogan kuzdogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is overall great thanks for writing!

I fixed some typos and left some small comments to what you have written. The only issue is the article is lacking a little the difference between a "full match" and a "partial match", and what contract metadata is. Partial match (how Etherscan verifies currently), which is what people usually refer to with verification, does not guarantee many things such as variable names, comments, NatSpec etc. It is only a verification that this piece of code compiles to the same bytecode. There are many parts in the text that this distinction is missing and I think it is important to convey this information and push people towards full verification, as well as introduce to them why and how the contract metadata is useful.

I can suggest writing the "How" and "Sourcify" parts from scratch, adding a section for contract metadata and full matches. Would you be okay with it and would it be possible in this PR to make changes directly?

Edit: I can fork from your repo and open a PR onto emmanuel-awosika:patch-12.


### User Safety {#user-safety}

With smart contracts, there’s usually a lot of money at stake. This calls for higher security guarantees and verification of a smart contract’s logic before using it. Without verification, malicious smart contracts can have [backdoors](https://www.trustnodes.com/2018/11/10/concerns-rise-over-backdoored-smart-contracts), controversial access control mechanisms, exploitable vulnerabilities, and other things that jeopardize user safety. The ability to verify source code for a smart contract reduces attack vectors and improves security for end-users.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also a requirement for a contract to be audited since auditors will need access to the smart contract source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edited the text to reflect your suggestion. Let me know your thoughts on it.

---

[Smart contracts](/developers/docs/smart-contracts/) are designed to be “trustless”, meaning users shouldn’t have to trust third parties (e.g., developers and companies) before interacting with a contract. As a requisite for trustleness, users and other developers must be able to verify a smart contract’s source code. Source code verification assures users and developers that the published contract code is the same code running at the contract address on the Ethereum blockchain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a distinction with Formal Verification:

"It is important to make the distinction between "source code verification" and "formal verification". The former, which will be explained in detail below, refers to verifying that the given source code of a smart contract in a high-level language (e.g. Solidity) compiles to the same bytecode to be executed at the contract address. The latter describes verifying the correctness of a smart contract, meaning the contract behaves as expected. . Although context-dependent, a verified contract usually refers to source code verification"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this now.


It is standard pratice in Solidity development to annotate source code files with inline comments using [NatSpec](https://docs.soliditylang.org/en/latest/natspec-format.html) (Ethereum Natutral Language Specification Format). NatSpec comments are human-readable descriptions of parts of a contract's code (e.g., contract functions and variables), which let users understand what happens with every contract interaction.

The problem is that unscruplous developers can deceive users by later inserting malicious code in the contract and changing comments before compiling the source. Source verification can prevent this by checking if comments in the published source file match those used during compilation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if fully verified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to: "Contract verification can prevent this by checking if comments in the published source file match those used during compilation, although this depends on whether the contract was fully or partially verified.". Does that work?

Copy link
Member

@kuzdogan kuzdogan Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should better distinguish code vs. comment. Because even a malicious actor can't straightforwardly add malicious code to a verified contract, because it would change the bytecode, but comments can be freely edited with partial matches. (Although there was a now fixed way to do this in Etherscan, because of how they parse the metadata hash. Check this out if you're interested.)

I'll suggest some changes to this in my PR to yours.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I included that article when discussing how Etherscan fails to check sameness of contract metadata (I assume metadata includes comments, but the text doesn't expatiate on it). But I think I made a mistake by referring to comments as "malicious code". Here's the text:

However, Etherscan's contract verification has a drawback: it fails to compare the metadata hash of the on-chain bytecode and recompiled bytecode. The Solidity compiler typically adds a hash of the contract metadata (stored on IPFS or Swarm) to the deployed bytecode. This is done to allow anyone retrieve the metadata independently and verify that contents of the contract's metadata remained the same before and after compilation.

Because Etherscan cannot check if the metadata changed or not, users must trust that those details remained consistent in the deployed code. But, as explained earlier, this opens the door for inserting malicious code in contract bytecode and can lead to malicious contract execution.

Looking forward to seeing your changes; I could learn a thing or two.

@emmanuel-awosika
Copy link
Contributor Author

This is overall great thanks for writing!

I fixed some typos and left some small comments to what you have written. The only issue is the article is lacking a little the difference between a "full match" and a "partial match", and what contract metadata is. Partial match (how Etherscan verifies currently), which is what people usually refer to with verification, does not guarantee many things such as variable names, comments, NatSpec etc. It is only a verification that this piece of code compiles to the same bytecode. There are many parts in the text that this distinction is missing and I think it is important to convey this information and push people towards full verification, as well as introduce to them why and how the contract metadata is useful.

I can suggest writing the "How" and "Sourcify" parts from scratch, adding a section for contract metadata and full matches. Would you be okay with it and would it be possible in this PR to make changes directly?

Edit: I can fork from your repo and open a PR onto emmanuel-awosika:patch-12.

Hey @kuzdogan, thanks for the feedback. I think it's a great idea to expand on differences between full vs partial matches. I only touched on it briefly (e.g., by explaining that Etherscan doesn't verify metadata hashes), but the concept could use more explanation.

I think you should be able to edit the PR directly. I'm still getting to grips with using GitHub, so I don't know how to merge PRs from my repo yet. That said, if you can't make changes, let me know, and I'll find a way around it.

Edits to incorporate feedback.
@kuzdogan
Copy link
Member

I can't edit the PR directly but a PR is basically a merge req. from one branch to another so if you make changes to the suggesting branch, the PR will update too. That's why I will be forking your repo and branch, and will open a PR onto yours. Once you merge them on emanuel-awosika:patch-12, it will be reflected in this PR.

@emmanuel-awosika
Copy link
Contributor Author

I can't edit the PR directly but a PR is basically a merge req. from one branch to another so if you make changes to the suggesting branch, the PR will update too. That's why I will be forking your repo and branch, and will open a PR onto yours. Once you merge them on emanuel-awosika:patch-12, it will be reflected in this PR.

Alright, then. I'll look out for any merge requests.

@emmanuel-awosika
Copy link
Contributor Author

emmanuel-awosika commented Jul 12, 2022

Hey @kuzdogan. I created a new folder for the source code verification page and moved the content to a new index.md file. I also added the image and replaced the external link with a link to the image in the folder. Does that work for you?


[Deploying a smart contract on Ethereum](/developers/docs/smart-contracts/deploying/) requires sending a transaction with a data payload (compiled bytecode) to a special address. The data payload is generated by compiling the source code, plus the [constructor arguments](https://docs.soliditylang.org/en/v0.8.14/contracts.html#constructor) of the contract instance appended to the data payload in the transaction. Compilation is deterministic, meaning it always produces the same output (i.e., contract bytecode) if the same source files, and compilation settings (e.g. compiler version, optimizer) are used.

![](https://github.com/emmanuel-awosika/ethereum-org-website/blob/0df62bc5ef7420929f079eb3174f1d69e5a2e3d3/src/content/developers/docs/smart-contracts/source-code-verification/Source%20code%20verification.png)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there needs to be a relative link here. Check Accounts page for the example: https://github.com/ethereum/ethereum-org-website/blob/dev/src/content/developers/docs/accounts/index.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also adding a higher resolution image:
source-code-verification

@kuzdogan
Copy link
Member

Looks good! Just one small thing about the image and we are good to go 👍

Couple of notes for reviewers:

@samajammin
Copy link
Member

samajammin commented Jul 12, 2022

Amazing, thanks for the reviews @kuzdogan! This is looking awesome @emmanuel-awosika.

@kuzdogan to your question:

I couldn't find this page in the generated Gatsby build at https://build-c0382175-68d5-49dc-9eb8-e53324852482.gtsb.io/en/developers/docs/smart-contracts/ maybe I'm missing something.

The page is located here:
https://ethereumorgwebsitedev01-emmanuelawosikaethereumor29708.gtsb.io/en/developers/docs/smart-contracts/source-code-verification/
But to your point, the page isn't yet linked in the sidebar of the docs. I'll quickly add that.

Edit: added the the page to the sidenav 15d92c5

@@ -0,0 +1,99 @@
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick/suggestion - could we shorten this directory name to:
src/content/developers/docs/smart-contracts/verifying/index.md?

This would create the page ethereum.org/en/developers/docs/smart-contracts/verifying/, keeping it consistent with others e.g.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, source-code-verification is more explicit (clearly separates it from formal verification of smart contrcts), so this isn't a very strong opinion. I just like the formatting consistency 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good for consistency as you suggest. As mentioned in the content, verifying usually refers to source code verification, plus we make the distinction with the formal v. at the very beginning. So I'd say 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we have a dilemma here. If we didn't have a page on formal verification, then "verifying" would be a good directory name for the page. The best suggestion I can make is using "source-verifying" for this page and "formal verifying" for the page on formal verification.

Also, @kuzdogan, I pushed a new commit to tweak the numbered list outlining instructions for verifying contracts. The initial list didn't put spaces between items, so it showed up as a paragraph in the preview.

@samajammin
Copy link
Member

I fixed the image reference & replaced it with the higher res version that @kuzdogan shared. I think this is now good to go! 🚀

Fixed the listing under "How to verify source code for contracts".
@emmanuel-awosika
Copy link
Contributor Author

I fixed the image reference & replaced it with the higher res version that @kuzdogan shared. I think this is now good to go! 🚀

Thank you, @samajammin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants